Subject: fix for vulnerability CVE-2011-3343 for OpenTTD 0.7.0 - 0.7.5 (Multiple buffer overflows in validation of external data)
From: OpenTTD developer team <info@openttd.org>
Origin: backport, https://github.com/OpenTTD/OpenTTD/commit/6c7cbb1 https://github.com/OpenTTD/OpenTTD/commit/65637d8 https://github.com/OpenTTD/OpenTTD/commit/73624ab
Bug: https://github.com/OpenTTD/OpenTTD/issues/4746 https://github.com/OpenTTD/OpenTTD/issues/4747

In multiple places external data isn’t properly checked before allocating
memory, which could lead to buffer overflows and arbitrary code execution.

These bugs are only exploitable locally by providing OpenTTD with
invalid/manipulated images, sounds or fonts. This means an attacker
either needs local access or has to trick an user into loading a
manipulated image into OpenTTD. This is especially a concern with
BMP files loaded as heightmaps.

All except one vulnerability are caused by improper validation of input
data prior to allocating memory buffers. It is possible to force allocation
of a too small buffer and thus out-of-bounds writes by causing an integer
overflow. Additionally in RLE-compressed BMP images, it is possible to
write arbitrary data outside the allocated buffer.

No patch for releases before 0.3.1 is provided, as this versions are
unsupported since a long time and would require larger changes not worth
the effort.


diff --git src/bmp.cpp src/bmp.cpp
index ae277d7..a839a5c 100644
--- src/bmp.cpp
+++ src/bmp.cpp
@@ -135,6 +135,7 @@ static inline bool BmpRead4Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			switch (c) {
 			case 0: // end of line
 				x = 0;
+				if (y == 0) return false;
 				pixel = &data->bitmap[--y * info->width];
 				break;
 			case 1: // end of bitmap
@@ -145,7 +146,7 @@ static inline bool BmpRead4Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			case 2: // delta
 				x += ReadByte(buffer);
 				i = ReadByte(buffer);
-				if (x >= info->width || (y == 0 && i > 0)) return false;
+				if (x >= info->width || i > y) return false;
 				y -= i;
 				pixel = &data->bitmap[y * info->width + x];
 				break;
@@ -218,6 +219,7 @@ static inline bool BmpRead8Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			switch (c) {
 			case 0: // end of line
 				x = 0;
+				if (y == 0) return false;
 				pixel = &data->bitmap[--y * info->width];
 				break;
 			case 1: // end of bitmap
@@ -228,13 +230,16 @@ static inline bool BmpRead8Rle(BmpBuffer *buffer, BmpInfo *info, BmpData *data)
 			case 2: // delta
 				x += ReadByte(buffer);
 				i = ReadByte(buffer);
-				if (x >= info->width || (y == 0 && i > 0)) return false;
+				if (x >= info->width || i > y) return false;
 				y -= i;
 				pixel = &data->bitmap[y * info->width + x];
 				break;
 			default: // uncompressed
-				if ((x += c) > info->width) return false;
-				for (i = 0; i < c; i++) *pixel++ = ReadByte(buffer);
+				for (i = 0; i < c; i++) {
+					if (EndOfBuffer(buffer) || x >= info->width) return false;
+					*pixel++ = ReadByte(buffer);
+					x++;
+				}
 				/* Padding for 16 bit align */
 				SkipBytes(buffer, c % 2);
 				break;
diff --git src/fontcache.cpp src/fontcache.cpp
index 1c1095b..bd3c75e 100644
--- src/fontcache.cpp
+++ src/fontcache.cpp
@@ -721,6 +721,9 @@ static bool GetFontAAState(FontSize size)
 	width  = max(1, slot->bitmap.width + (size == FS_NORMAL));
 	height = max(1, slot->bitmap.rows  + (size == FS_NORMAL));
 
+	/* Limit glyph size to prevent overflows later on. */
+	if (width > 256 || height > 256) usererror("Font glyph is too large");
+
 	/* FreeType has rendered the glyph, now we allocate a sprite and copy the image into it */
 	sprite.AllocateData(width * height);
 	sprite.width = width;
diff --git src/heightmap.cpp src/heightmap.cpp
index 04207cd..790847e 100644
--- src/heightmap.cpp
+++ src/heightmap.cpp
@@ -136,6 +136,14 @@ static bool ReadHeightmapPNG(char *filename, uint *x, uint *y, byte **map)
 		return false;
 	}
 
+	/* Check if image dimensions don't overflow a size_t to avoid memory corruption. */
+	if ((uint64)info_ptr->width * info_ptr->height >= (size_t)-1) {
+		ShowErrorMessage(STR_ERROR_HEIGHTMAP_TOO_LARGE, STR_PNGMAP_ERROR, 0, 0);
+		fclose(fp);
+		png_destroy_read_struct(&png_ptr, &info_ptr, NULL);
+		return false;
+	}
+
 	if (map != NULL) {
 		*map = MallocT<byte>(info_ptr->width * info_ptr->height);
 		ReadHeightmapPNGImageData(*map, png_ptr, info_ptr);
@@ -237,6 +245,14 @@ static bool ReadHeightmapBMP(char *filename, uint *x, uint *y, byte **map)
 		return false;
 	}
 
+	/* Check if image dimensions don't overflow a size_t to avoid memory corruption. */
+	if ((uint64)info.width * info.height >= (size_t)-1 / (info.bpp == 24 ? 3 : 1)) {
+		ShowErrorMessage(STR_ERROR_HEIGHTMAP_TOO_LARGE, STR_BMPMAP_ERROR, 0, 0);
+		fclose(f);
+		BmpDestroyData(&data);
+		return false;
+	}
+
 	if (map != NULL) {
 		if (!BmpReadBitmap(&buffer, &info, &data)) {
 			ShowErrorMessage(STR_BMPMAP_ERR_IMAGE_TYPE, STR_BMPMAP_ERROR, 0, 0);
diff --git src/lang/english.txt src/lang/english.txt
index f0fd2d8..22525fd 100644
--- src/lang/english.txt
+++ src/lang/english.txt
@@ -1529,6 +1529,8 @@ STR_PNGMAP_ERR_MISC                                             :{WHITE}...somet
 STR_BMPMAP_ERROR                                                :{WHITE}Can't load landscape from BMP...
 STR_BMPMAP_ERR_IMAGE_TYPE                                       :{WHITE}...could not convert image type.
 
+STR_ERROR_HEIGHTMAP_TOO_LARGE                                   :{WHITE}... image is too large
+
 ##id 0x0800
 STR_0800_COST                                                   :{TINYFONT}{RED}Cost: {CURRENCY}
 STR_0801_COST                                                   :{RED}Cost: {CURRENCY}
diff --git src/openttd.cpp src/openttd.cpp
index 70525cd..9ccf7fe 100644
--- src/openttd.cpp
+++ src/openttd.cpp
@@ -540,11 +540,12 @@ int ttd_main(int argc, char *argv[])
 
 	/*
 	 * The width and height must be at least 1 pixel and width times
-	 * height must still fit within a 32 bits integer, this way all
-	 * internal drawing routines work correctly.
+	 * height times bytes per pixel must still fit within a 32 bits
+	 * integer, even for 32 bpp video modes. This way all internal
+	 * drawing routines work correctly.
 	 */
-	_cur_resolution.width  = ClampU(_cur_resolution.width,  1, UINT16_MAX);
-	_cur_resolution.height = ClampU(_cur_resolution.height, 1, UINT16_MAX);
+	_cur_resolution.width  = ClampU(_cur_resolution.width,  1, UINT16_MAX / 2);
+	_cur_resolution.height = ClampU(_cur_resolution.height, 1, UINT16_MAX / 2);
 
 #if defined(ENABLE_NETWORK)
 	if (dedicated_host) snprintf(_settings_client.network.server_bind_ip, sizeof(_settings_client.network.server_bind_ip), "%s", dedicated_host);
diff --git src/script/squirrel_helper.hpp src/script/squirrel_helper.hpp
index 2310b70..59a47b1 100644
--- src/script/squirrel_helper.hpp
+++ src/script/squirrel_helper.hpp
@@ -111,6 +111,9 @@
 
 	template <> inline Array      *GetParam(ForceType<Array *>,      HSQUIRRELVM vm, int index, SQAutoFreePointers *ptr)
 	{
+		/* Sanity check of the size. */
+		if (sq_getsize(vm, index) > UINT16_MAX) throw sq_throwerror(vm, _SC("an array used as parameter to a function is too large"));
+
 		SQObject obj;
 		sq_getstackobj(vm, index, &obj);
 		sq_pushobject(vm, obj);
diff --git src/sound.cpp src/sound.cpp
index c487401..e6ed123 100644
--- src/sound.cpp
+++ src/sound.cpp
@@ -105,7 +105,8 @@ static bool SetBankSource(MixerChannel *mc, const FileEntry *fe)
 {
 	assert(fe != NULL);
 
-	if (fe->file_size == 0) return false;
+	/* Check for valid sound size. */
+	if (fe->file_size == 0 || fe->file_size > ((size_t)-1) - 2) return false;
 
 	int8 *mem = MallocT<int8>(fe->file_size);
 
diff --git src/sound/win32_s.cpp src/sound/win32_s.cpp
index 776158a..24d9c31 100644
--- src/sound/win32_s.cpp
+++ src/sound/win32_s.cpp
@@ -61,6 +61,7 @@ static void CALLBACK waveOutProc(HWAVEOUT hwo, UINT uMsg, DWORD_PTR dwInstance,
 	wfex.nAvgBytesPerSec = wfex.nSamplesPerSec * wfex.nBlockAlign;
 
 	_bufsize = GetDriverParamInt(parm, "bufsize", (GB(GetVersion(), 0, 8) > 5) ? 2048 : 1024);
+	_bufsize = min(_bufsize, UINT16_MAX);
 
 	if (waveOutOpen(&_waveout, WAVE_MAPPER, &wfex, (DWORD_PTR)&waveOutProc, 0, CALLBACK_FUNCTION) != MMSYSERR_NOERROR)
 		return "waveOutOpen failed";
diff --git src/spriteloader/png.cpp src/spriteloader/png.cpp
index f99237d..fb8072f 100644
--- src/spriteloader/png.cpp
+++ src/spriteloader/png.cpp
@@ -101,7 +101,17 @@ static bool LoadPNG(SpriteLoader::Sprite *sprite, const char *filename, uint32 i
 
 		sprite->height = info_ptr->height;
 		sprite->width  = info_ptr->width;
+		/* Check if sprite dimensions aren't larger than what is allowed in GRF-files. */
+		if (info_ptr->height > 255 || info_ptr->width > UINT16_MAX) {
+			png_destroy_read_struct(&png_ptr, &info_ptr, &end_info);
+			return false;
+		}
 		sprite->AllocateData(sprite->width * sprite->height);
+	} else if (sprite->height != info_ptr->height || sprite->width != info_ptr->width) {
+		/* Make sure the mask image isn't larger than the sprite image. */
+		DEBUG(misc, 0, "Ignoring mask for SpriteID %d as it isn't the same dimension as the masked sprite", id);
+		png_destroy_read_struct(&png_ptr, &info_ptr, &end_info);
+		return true;
 	}
 
 	bit_depth  = png_get_bit_depth(png_ptr, info_ptr);
@@ -109,6 +119,7 @@ static bool LoadPNG(SpriteLoader::Sprite *sprite, const char *filename, uint32 i
 
 	if (mask && (bit_depth != 8 || colour_type != PNG_COLOR_TYPE_PALETTE)) {
 		DEBUG(misc, 0, "Ignoring mask for SpriteID %d as it isn't a 8 bit palette image", id);
+		png_destroy_read_struct(&png_ptr, &info_ptr, &end_info);
 		return true;
 	}
 

